-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(core-library): do not create pr when a pr already exists for a ref #66
fix(core-library): do not create pr when a pr already exists for a ref #66
Conversation
Codecov Report
@@ Coverage Diff @@
## master #66 +/- ##
==========================================
+ Coverage 85.96% 86.11% +0.15%
==========================================
Files 14 14
Lines 1190 1203 +13
Branches 83 81 -2
==========================================
+ Hits 1023 1036 +13
Misses 166 166
Partials 1 1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this handles the case when essentially there is an existing branch. Is this not the same case as the force option?
Correct. Basically if there is an existing PR for the branch, do not create a new PR, re-use the old one. GitHub PRs just point to a reference head which are applied on-top of some reference head, typically master, and so when the reference gets updated the PR automatically gets updated as well. |
test/pr.ts
Outdated
}); | ||
}); | ||
|
||
it('Invokes octokit pull create when there are similar refs with pull requests open, but not exact refs: extra number', async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can nest describe blocks - many of these tests share the same setup for the case of an already existing PR.
describe('when there are similar refs with pull requests open', async () => {
beforeEach(() => {
// set up sinon for list pull request
});
it('should create another pull request if it's not the exact ref', async () => {
});
});
'./fixtures/list-pulls-response.json' | ||
); | ||
afterEach(() => { | ||
sandbox.restore(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be already handled by the outer describe blocks's afterEach
which is restoring the sandbox.
Recall that GitHub PRs just point to a reference head which are applied on-top of some base reference head, typically master, and so when the reference gets updated the PR automatically gets updated as well.
There may be scenarios where a pull request is already created for a ref, and a user will re-invoke code-suggester create pr using the same ref. If the
force
flag/parameter is true, a new ref is not created. Instead code-suggester create pr will force-update the existing reference. If we try to re-create the a pr for the same reference, there will be a duplication issue. Hence, if there is an existing pr, just update the ref, and GitHub will just adjust UI display accordingly to the reference HEAD data. However, ifforce
flag/parameter is false, code-suggester create pr will fail when trying to create the reference because there is a duplicate reference.TL;DR
When there is an existing pr for the a reference and code-suggester create pr is re-invoked, do not create a new pr. Instead, re-use the old one and update the ref.
Closes #17